Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sensor / emon revamp (wip?) #2213

Merged
merged 14 commits into from
Apr 7, 2020
Merged

Sensor / emon revamp (wip?) #2213

merged 14 commits into from
Apr 7, 2020

Conversation

mcspr
Copy link
Collaborator

@mcspr mcspr commented Apr 6, 2020

@irmishappy (sorry for the delay)

Resolves ... (TODO: find all related issues)

  • Update sensor definitions to support a generic way to store energy values
  • Update sensor conversion code to deal with units and not magnitudes (ee53849)
  • Update sensor unit definitions to use a more generic way of defining unit IDs.
  • Reset energy value based on index through external means

While this solves the energy conversion issues and we are finally seeing the real value, what I don't really like:

  • KilowattHour and WattHour are separate enum tags, thus sort-of are different types altogether
  • Conversion code in Energy object should probably use some generic 'ratio' calculation? (https://en.cppreference.com/w/cpp/numeric/ratio/ratio)
  • We are still using runtime checks to do calculations and depend that sensor outputs only one specific value type.

I have found some alternative systems:
https://github.com/nholthaus/units/
Which implement unit as strongly typed class that can be used to enforce our runtime checks at compile time. Non-trivial part is that I am not really sure what structure the code must use to implement strongly typed getters. We can do something like:

class BaseSensor {
    virtual units::energy::KilowattHour getEnergy(unsigned char index) { return 0; } 
    virtual units::temperature::Celcius getTemperature(unsigned char index) { return 0; } 
};

And we still need to check specific run-time property of the sensor to know what getter outputs correct data. We would get rid of conversion stuff though, it would magically do it all by itself.
(Note that I am not sure about the underlying type / value stuff and whether we can hit some limits there, units.h source is pretty cryptic :/)

mcspr added 14 commits April 4, 2020 04:39
Fix longstanding issue with storing energy data in J (Ws).
Simplify process of switching from J (Ws) to kWh.

Adjust every emon sensor to use BaseEmonSensor base class
and every analog sensor to use BaseAnalogSensor.
Allow shared interface across similar sensors.
We never use base members, we don't need them present.
(or expect them to be initialized, somehow)

General pattern is to wait until ctor body to set any properties.
@mcspr mcspr merged commit cae50fa into xoseperez:dev Apr 7, 2020
@mcspr mcspr deleted the sns/energy-api branch April 7, 2020 21:37
@mcspr
Copy link
Collaborator Author

mcspr commented Apr 7, 2020

Also bookmarking something from https://github.com/discover:
https://github.com/LLNL/units#alternatives

At least one of the options may be reasoned with a bit more easily:
https://github.com/mpusz/units
https://mpusz.github.io/units/framework/basic_concepts.html
What will not be really obvious is how to port c++20 / c++23 code to ours c++11 (or, potentially, c++14)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant